Skip to content

Remove dead code related to java compatibility now that Java min version is 21 #20143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Remove dead code related to java compatibility now that Java min version is 21 #20143

wants to merge 1 commit into from
+3 −17

Conversation

@cwperks
Copy link
Member

@cwperks cwperks commented Dec 1, 2025 •
edited by coderabbitai bot
Loading

Description

Remove dead code related to java compatibility now that Java min version is 21

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Summary by CodeRabbit

  • Chores
    • Improved build consistency by unconditionally applying Java release flags to compilation tasks.
    • Extended release flag application to Groovy compilation for consistency.
    • Simplified test configuration by standardizing locale provider settings across all Java versions.
    • Enhanced Javadoc generation with HTML5 enablement.

✏️ Tip: You can customize this high-level summary in your review settings.

@cwperks cwperks requested a review from a team as a code owner December 1, 2025 22:00
@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025 •
edited
Loading

Walkthrough

These changes simplify build configuration by removing Java version-specific runtime guards. The Java plugin now unconditionally applies release flags to Java and Groovy compilation and always enables HTML5 in Javadoc. The test base plugin removes version-dependent locale provider logic, always configuring it uniformly regardless of runtime version.

Changes

Cohort / File(s) Change Summary
Java and Javadoc compilation configuration
buildSrc/src/main/java/org/opensearch/gradle/OpenSearchJavaPlugin.java		 Removed runtime-version guards and now unconditionally apply release flags to Java compilation tasks via compileOptions.release, extend same behavior to Groovy compilation (GroovyCompile), and always enable html5 in Javadoc CoreJavadocOptions
Test configuration simplification
buildSrc/src/main/java/org/opensearch/gradle/OpenSearchTestBasePlugin.java		 Removed JavaVersion import and version-based conditional logic; now unconditionally set java.locale.providers to "SPI,CLDR" and removed per-version jvmArgs handling for older runtimes

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Both changes are straightforward simplifications removing version-dependent conditional branches
  • No new logic or complex interactions introduced
  • Key consideration: verify that removing version guards doesn't inadvertently break builds on older or newer Java versions by losing necessary configuration fallbacks

Poem

🐰 No more version checks to weigh us down,
Release flags now wear the crown!
Groovy, Java, Javadoc so bright—
Simplified configs, set it right! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive The description lacks concrete details about the specific changes made and omits the 'Related Issues' section from the template. Add details about which specific code was removed and reference the issue number (e.g., 'Resolves #...'). Ensure all template sections are addressed.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: removing dead code related to Java compatibility now that the minimum Java version is 21.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 859f7c3 and 450f50f.

📒 Files selected for processing (2)
  • buildSrc/src/main/java/org/opensearch/gradle/OpenSearchJavaPlugin.java (2 hunks)
  • buildSrc/src/main/java/org/opensearch/gradle/OpenSearchTestBasePlugin.java (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: gradle-check
  • GitHub Check: Analyze (java)
  • GitHub Check: precommit (21, macos-15)
  • GitHub Check: precommit (25, ubuntu-24.04-arm)
  • GitHub Check: precommit (25, macos-15-intel)
  • GitHub Check: precommit (21, windows-2025, true)
  • GitHub Check: precommit (21, ubuntu-24.04-arm)
  • GitHub Check: precommit (25, windows-latest)
  • GitHub Check: precommit (25, macos-15)
  • GitHub Check: precommit (21, windows-latest)
  • GitHub Check: precommit (21, ubuntu-latest)
  • GitHub Check: precommit (25, ubuntu-latest)
  • GitHub Check: precommit (21, macos-15-intel)
  • GitHub Check: assemble (25, ubuntu-latest)
  • GitHub Check: assemble (25, ubuntu-24.04-arm)
  • GitHub Check: assemble (21, ubuntu-latest)
  • GitHub Check: assemble (25, windows-latest)
  • GitHub Check: assemble (21, windows-latest)
  • GitHub Check: assemble (21, ubuntu-24.04-arm)
  • GitHub Check: detect-breaking-change
🔇 Additional comments (4)
buildSrc/src/main/java/org/opensearch/gradle/OpenSearchJavaPlugin.java (3)

177-177: LGTM! Release flag now applied unconditionally.

Unconditionally setting the release flag for Java compilation is the correct approach for Java 21+ and ensures proper cross-compilation behavior.

180-183: Release flag applied to Groovy compilation.

The change correctly extends the unconditional release flag to Groovy compilation tasks. Note the existing TODO on line 181 questions whether this should apply to Groovy at all—this is a pre-existing concern, not introduced by this change.

271-271: LGTM! HTML5 Javadoc now enabled unconditionally.

Unconditionally enabling HTML5 in Javadoc is correct for Java 21+, as HTML5 has been the standard format for several Java versions.

buildSrc/src/main/java/org/opensearch/gradle/OpenSearchTestBasePlugin.java (1)

108-108: LGTM! Locale provider configuration simplified for Java 21+.

The unconditional setting of java.locale.providers to "SPI,CLDR" is correct for Java 21+ and simplifies the test configuration appropriately. To ensure completeness, verify that no other build configuration files still reference the removed version checks by searching for remaining JavaVersion runtime checks, --illegal-access flags, or conflicting locale provider configurations.

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2025

❌ Gradle check result for 450f50f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions
Copy link
Contributor

github-actions bot commented Dec 2, 2025

✅ Gradle check result for 450f50f: SUCCESS

@codecov
Copy link

codecov bot commented Dec 2, 2025

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.18%. Comparing base (0197084) to head (450f50f).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...va/org/opensearch/gradle/OpenSearchJavaPlugin.java 0.00% 2 Missing ⚠️
...rg/opensearch/gradle/OpenSearchTestBasePlugin.java 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main   #20143      +/-   ##
============================================
- Coverage     73.23%   73.18%   -0.05%     
- Complexity    71621    71633      +12     
============================================
  Files          5787     5787              
  Lines        327853   327848       -5     
  Branches      47218    47214       -4     
============================================
- Hits         240090   239930     -160     
- Misses        68505    68671     +166     
+ Partials      19258    19247      -11

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

skip-changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@cwperks